Thanks for the PR!

Although this technically fixes the #2156, I'd ask if you want to find the culprit here. As this is very strange to expect errors after explicitly checking that win_id is valid.

Writing a test case would possible make this easier. My guess is that this has to do with 'mini.files' being default explorer.

Yes, without mini.files the problem does not occur.

My guess is that mini.files is "in progress", creating or refreshing its buffers. I just don't understand why it doesn't happen when mini.starter is visible.

I will look into it.

I modified mini.files a little to prevent an explicit vim.api.nvim_buf_delete:

H.track_dir_edit = function(data)
  -- Make early returns
  if vim.api.nvim_get_current_buf() ~= data.buf then return end

  if vim.b.minifiles_processed_dir then
    -- Smartly delete directory buffer if already visited
    local alt_buf = vim.fn.bufnr('#')
    if alt_buf ~= data.buf and vim.fn.buflisted(alt_buf) == 1 then vim.api.nvim_win_set_buf(0, alt_buf) end
    return -- COMMENTED return vim.api.nvim_buf_delete(data.buf, { force = true })
  end

  local path = vim.api.nvim_buf_get_name(0)
  if vim.fn.isdirectory(path) ~= 1 then return end

  -- Make directory buffer disappear when it is not needed
  vim.bo.bufhidden = 'wipe'
  vim.b.minifiles_processed_dir = true

  -- Open directory without history
  vim.schedule(function() MiniFiles.open(path, false) end)
end

I added some print statements. The scenario: Open nvim with init.lua, press fv, select visited directory entry in picker, enter...

mini.files in track dir for data.buf 1
mini.files in track dir, no early return
mini.files in track dir, return, not a directory /home/user/.config/repro/init.lua
mini.pick in default chose
mini.files in track dir for data.buf 3
mini.files in track dir, no early return
mini.files still in track dir, marking vim.b.minifiles_processed_dir
mini.pick advance before stop -- NOTE: mini.files does not yet appear....
mini.pick set curwin
mini.files in track dir for data.buf 3
mini.files in track dir, no early return
mini.files in track dir, has vim.b.minifiles_processed_dir, alt buf is 1
---
--- Without modification, crashes here...
---
--- Happy path:
mini.files in track dir for data.buf 4
mini.files in track dir, no early return
mini.files in track dir, return, not a directory minifiles://4//home/user/.config/repro

The data.buf 3 must be the mini.files buffer. The problem occurs after mini.pick sets the current window in its picker_stop. As a result, mini.files H.track activates again, this time with vim.b.minifiles_processed_dir set. Without the modification, data.buf 3 is actively deleted... This also explains why there is no error when the dashboard is active. That buffer is unlisted...

So, the fix could also be in mini.files, by not deleting the buffer and just return instead. Perhaps the explicit delete is not necessary anyways, because of vim.bo.bufhidden = 'wipe'

I think its best to fix in mini.files, to not crash on an external vim.api.set_current_win. All tests pass.

So, the fix could also be in mini.files, by not deleting the buffer and just return instead. Perhaps the explicit delete is not necessary anyways, because of vim.bo.bufhidden = 'wipe'

I think its best to fix in mini.files, to not crash on an external vim.api.set_current_win. All tests pass.

The buffer deleting is for the following use case:

• :edit . (or any other directory).
• Press q. Without deleting there is a buffer for the current directory if there is no alternative buffer. Removing nvim_buf_delete() leaves the buffer for the target directory.

It is probably not a big deal and a rare use case, but at least trying to account for that is reasonable.

And, of course, needs tests both here and 'mini.extra'.

I unfortunately cannot reproduce the buffer deleting scenario you described. Testing with ls! I don't see dangling buffers after the fix.

Still todo: A test in mini.extra

I unfortunately cannot reproduce the buffer deleting scenario you described. Testing with ls! I don't see dangling buffers after the fix.

1. Create '~/.config/nvim-repro/init.lua':

   -- Clone latest 'mini.nvim' (requires Git CLI installed)
   vim.cmd('echo "Installing `mini.nvim`" | redraw')
   local mini_path = vim.fn.stdpath('data') .. '/site/pack/deps/start/mini.nvim'
   local clone_cmd = { 'git', 'clone', '--depth=1', 'https://github.com/nvim-mini/mini.nvim', mini_path }
   vim.fn.system(clone_cmd)
   vim.cmd('echo "`mini.nvim` is installed" | redraw')
   
   vim.cmd('packadd mini.nvim')
   
   require('mini.files').setup()

2. Apply the following patch to '~/.local/share/nvim-repro/site/pack/deps/start/mini.nvim/':

   diff --git a/lua/mini/files.lua b/lua/mini/files.lua
   index 470cb20..ace4fc6 100644
   --- a/lua/mini/files.lua
   +++ b/lua/mini/files.lua
   @@ -1383,7 +1383,7 @@ H.track_dir_edit = function(data)
        -- Smartly delete directory buffer if already visited
        local alt_buf = vim.fn.bufnr('#')
        if alt_buf ~= data.buf and vim.fn.buflisted(alt_buf) == 1 then vim.api.nvim_win_set_buf(0, alt_buf) end
   -    return vim.api.nvim_buf_delete(data.buf, { force = true })
   +    return
      end
    
      local path = vim.api.nvim_buf_get_name(0)

3. NVIM_APPNAME=nvim-repro nvim

4. :edit . and press q.

5. :ls shows a buffer named ., although it should not be there. To make it more visible, the previous step can be :edit ~/.config/nvim-repro/.

... To make it more visible, the previous step can be :edit ~/.config/nvim-repro/.

Yes, that clarifies..)

@abeldekat, couple of quick notes before I'll get to this tomorrow:

• Please, minimize number of lines in 'files.lua' diff. I think it should be possible to just move the return inside earlier if ... then. No need for else here.
• Ignore some 'test_files.lua' fails on Nightly. It will require changes after recent Nightly changes.
• Currently tests in this PR are a bit too verbose. For example:
  • There are comment details that will become outdated after the fix.
  • I think 'test_extra.lua' test can just test that there is no error and that 'mini.files' is shown. No need for screenshot testing here. Especially since it requires mocking window title.

I think 'test_extra.lua' test can just test that there is no error and that 'mini.files' is shown. No need for screenshot testing here. Especially since it requires mocking window title.

I am trying to test for the presence of errors. Unfortunately, vim.v.errmsg is empty. The expect_screenshot does show the error situation, whereby the picker is not closed. However, the error text that pops up in a real session is not shown. After the fix, the expect_screenshot shows the correct situation: picker closed, files open...

I am trying to test for the presence of errors. Unfortunately, vim.v.errmsg is empty. The expect_screenshot does show the error situation, whereby the picker is not closed. However, the error text that pops up in a real session is not shown. After the fix, the expect_screenshot shows the correct situation: picker closed, files open...

Usually a test for error is expect.error(function() ... end, 'error pattern').

But the 'test_extra.lua' should test for absence of errors. Which is expect.no_error(function() ... end) or just testing some equality without the test failing.

Thanks again for the PR!

I'll merge in temporary branch and adjust some things.

The variation of this should now be on latest main. See 342fa09 for comparison.

The big difference (from my earlier comments as well) is that it seemed like a good idea to check 'mini.pick' directly that it works when choosing the directory path. And not one picker in 'mini.extra'. The actual 'mini.pick' testing approach also isn't exactly super robust or clean (screenshot might have been better), but seems good enough: it breaks without this PR's code change and passes otherwise.

The variation of this should now be on latest main. See 342fa09 for comparison.

Great! Thank you, also for the advice in the review.

Regarding the original test in test_extra, I did know to test for expect.no_error(function() ... end). But I first wanted to see the test fail on the old code without the return, using expect.error. That did not work unfortunately, the test always succeeded. Anyways, the test in test_pick is much better.

But I first wanted to see the test fail on the old code without the return, using expect.error. That did not work unfortunately, the test always succeeded. Anyways, the test in test_pick is much better.

Yeah, the error that is present when run interactively did not propagate for some reason. That's why I had to resort to counting floating windows instead.